Skip to content

New feature branch#3

Open
KingsleyMcSimon wants to merge 31 commits intodevelopmentfrom
new_feature_branch
Open

New feature branch#3
KingsleyMcSimon wants to merge 31 commits intodevelopmentfrom
new_feature_branch

Conversation

@KingsleyMcSimon
Copy link
Copy Markdown
Owner

No description provided.

Copy link
Copy Markdown

@stratospherique stratospherique left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work implementing the suggested changes 👏 However, some methods doesn't meet with requirements. check below ⬇️

  • [REQUIRED] Fix linter errors. They should be ✔️

Methods issues

my_each
  • my_each returns an Enumerator if no block is given
      array = [1,2,5,8,7,98,4]
      p array.my_each.class # not Enumerator
    
my_each_with_index
  • my_each_with_index returns an enumerator if no block is given
      array = [1,2,5,8,7,98,4]
      p array.my_each_with_index.class # not Enumerator
    
my_select
  • my_select returns an array containing all elements of enum for which the given block returns a true value
      block = proc {|x| x>4}
      array = [1,2,5,8,7,98,4]
      p array.my_select(&block) == array.select(&block) # false
    
  • my_select returns an enumerator if no block is given
      array = [1,2,5,8,7,98,4]
      p array.my_select.class # not Enumerator
    
my_all?
  • my_all? when no block or argument is given returns true when none of the collection members are false or nil
      array = [1,2,5,4,7]
      p array.my_all? == array.all? # false
    
  • my_all? when a class is passed as an argument returns true if all of the collection is a member of such class
      array = [1,2,5,4,7]
      p array.my_all?(Integer) == array.all?(Integer) # false
    
  • my_all? when a Regex is passed as an argument returns true if all of the collection matches the Regex
      array = %w[dog door rod blade]
      p array.my_all?(/c/) == array.all?(/c/) # false
    
  • my_all? when a pattern other than Regex or a Class is given returns true if all of the collection matches the pattern
      array = %w[dog door rod blade]
      p array.my_all?(5) == array.all?(5) # false
    
my_any?
  • my_any? when no block or argument is given returns true if at least one of the collection is not false or nil
      array = [1,2,5,4,7]
      p array.my_any? == array.any? # false
    
  • my_any? when a class is passed as an argument returns true if at least one of the collection is a member of such class
      array = [1,2,5,4,7]
      p array.my_any?(Integer) == array.any?(Integer) # false
    
  • ( ] my_any? when a Regex is passed as an argument returns false if none of the collection matches the Regex
      array = %w[dog door rod blade]
      p array.my_any?(/c/) == array.any?(/c/) # false
    
  • my_any? when a pattern other than Regex or a Class is given returns false if none of the collection matches the pattern
      array = %w[dog door rod blade]
      p array.my_any?(5) == array.any?(5) # false
    
my_none?
  • my_none? when no block or argument is given returns true only if none of the collection members is true
      array = [1,2,5,4,7]
      p array.my_none? == array.none? # false
    
  • my_none? when a class is passed as an argument returns true if none of the collection is a member of such class
      array = [1,2,5,4,7]
      p array.my_none?(Integer) == array.none?(Integer) # false
    
  • my_none? when a Regex is passed as an argument returns true only if none of the collection matches the Regex
      array = %w[dog door rod blade]
      p array.my_none?(/c/) == array.none?(/c/) # false
    
  • my_none? when a pattern other than Regex or a Class is given returns true only if none of the collection matches the pattern
      array = %w[dog door rod blade]
      p array.my_none?(5) == array.none?(5) # false
    
my_count
  • my_count counts the number of items in enum that are equal to item if an argument is given
array = [7,8,9,5,1,0,2,4,0,1]
array.my_count(0) == array.count(0) # false
  • my_count counts the number of elements yielding a true value if a block is given
array = [1,2,8,9,7,5,6,3,11]
block = proc { |num| num < 5 }
array.my_count(&block) == array.count(&block) # false
my_map
  • my_map returns an Enumerator if no block is given
    array = [1,5,8,7,9,5,5,55,5]
    p array.my_map #error
    LocalJumpError:
        no block given (yield)
      # ./Enumerable_Methods.rb:89:in `block in my_map'
      # ./Enumerable_Methods.rb:12:in `my_each'
      # ./Enumerable_Methods.rb:88:in `my_map'
my_inject
  • my_inject combines all elements of enum by applying a binary operation, specified by a block
  • my_inject when a symbol is specified combines each element of the collection by applying the symbol as a named method
      array = [1,5,8,7,9,5,5,55,5]
      p array.my_inject(:+) 
      # return this errors 
      LocalJumpError:
          no block given (yield)
        # ./Enumerable_Methods.rb:100:in `block in my_inject'
        # ./Enumerable_Methods.rb:12:in `my_each'
        # ./Enumerable_Methods.rb:99:in `my_inject'
    

That's it ☺️ When finished, kindly resubmit another code review request 🙏

Happy coding ✌️
Ahmed Mahfoudh

Copy link
Copy Markdown

@Ispirett Ispirett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Status changes required

@KingsleyMcSimon great job so far there are just two more bugs 🐛 to take care of.

Also please take care of stickler problems.

end

#my_inject method
def my_inject(param = nil, param1 = nil)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My inject
words = [] #=> some words
search = proc { |memo, word| memo.length > word.length ? memo : word }
words.my_inject(&search)) == words.inject(&search)

And
array.my_inject(:+) == eq array.inject(:+)

gives me this error
NoMethodError:
undefined method `length' for 5..50:Range

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,
Please could you explain to me what you exactly mean by this review on this my inject method i have from this pull request: #3

Copy link
Copy Markdown

@whiz25 whiz25 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @KingsleyMcSimon
Nice try implementing the suggested changes by the previous reviewer. However, this PR is not ready for merge yet.
See my comments and implement the requested changes.
Meanwhile, please fix the stickler error.

Martin Afani

@whiz25
Copy link
Copy Markdown

whiz25 commented Nov 15, 2019

my_all?

  • when a class is passed as an argument returns true if all of the collection is a member of such class
  • when a Regex is passed as an argument returns true if all of the collection matches the Regex
  • when a pattern other than Regex or a Class is given returns true if all of the collection matches the pattern

my_any?

  • when a class is passed as an argument returns true if at least one of the collection is a member of such class
  • when a Regex is passed as an argument returns false if none of the collection matches the Regex
  • when a pattern other than Regex or a Class is given returns false if none of the collection matches the pattern

my_none?

  • when a class is passed as an argument returns true if none of the collection is a member of such class
  • when a Regex is passed as an argument returns true only if none of the collection matches the Regex
  • when a pattern other than Regex or a Class is given returns true only if none of the collection matches the pattern

my_inject

  • combines all elements of enum by applying a binary operation, specified by a block
  • when a symbol is specified combines each element of the collection by applying the symbol as a named method
  • when no block is given, it returns an enumerator. This can be achieved by return to_enum

Copy link
Copy Markdown

@stickler-ci stickler-ci bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your rubocop configuration output the following error:


@KingsleyMcSimon
Copy link
Copy Markdown
Owner Author

This is really an old project and i believe that all the recommended changes has been implemented and the stickler and rubocop has passed its checks.

Copy link
Copy Markdown

@Forison Forison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Status:: Changes required❌

🙋‍♂️ McSimon !!!

Good job on addressing most of the issue raised in the earlier review, here are a few things we have to do to get this project approved.

  • There should be a working my_any? identical and returns the same thing as ruby's any?. (Screenshot from Odin)

p [nil, true, 99].my_any?(Integer) should return same results as
p [nil, true, 99].any?(Integer)

  • There should be a working my_none? identical and returns the same thing as ruby's none?. (Screenshot from Odin)

p [1, 3.14, 42].my_none?(Float) should return same result as
p [1, 3.14, 42].none?(Float)

  • There should be working my_inject identical and returns the same thing as ruby's inject. (Screenshot from Odin)

p (5..10).my_inject { |sum, n| sum + n } should be equal to
p (5..10).inject { |sum, n| sum + n }

p (5..10).my_inject(1) { |product, n| product * n } should be equal to
p (5..10).my_inject(1) { |product, n| product * n }

longest = %w{ cat sheep bear }.my_inject do |memo, word|
   memo.length > word.length ? memo : word
 end
p longest

should be equal to

longest = %w{ cat sheep bear }.inject do |memo, word|
   memo.length > word.length ? memo : word
 end
p longest
  • Do not commit commented code in your file.
  • Kindly remove your test examples for the main file, you can place your test examples in a separate file and import it as when needed.

Kindly make the changes and submit the project for a re-review😎.

Happy coding👨‍💻.

Addo Forison

@KingsleyMcSimon
Copy link
Copy Markdown
Owner Author

@Forison. I have been able to implement the recommended changes on this PR but i think you may have to re-run these methods you asked me to work on: my_any?, my_none? and my_inject?. This is because it actually passes the required test. Thanks.

@Forison
Copy link
Copy Markdown

Forison commented Jul 4, 2020

@Forison. I have been able to implement the recommended changes on this PR but i think you may have to re-run these methods you asked me to work on: my_any?, my_none? and my_inject?. This is because it actually passes the required test. Thanks.

Hi Simons, I tried to reach you on slack but It seems you are not available for now. I will record a loom video and attached to this file make you appreciate the persistent errors.

Copy link
Copy Markdown

@Forison Forison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Status:: Changes required❌

🙋‍♂️ McSimon !!!

As I said in the comment I wanted to reach out to you via slack but you were not available, hence I have recorded a video to point the flaws to you.

  • Click the link to view flaws as pointed out in my previous review. watch this video

Kindly reach out to me on slack if the video is not clear.

Kindly make the changes and submit the project for a re-review😎.

Happy coding👨‍💻.

Addo Forison

@KingsleyMcSimon
Copy link
Copy Markdown
Owner Author

Hi TSE,
I have worked and implemented the required changes and the methods #my_any?, #my_inject and #my_none? now passes the test.

@davidmrtz-dev
Copy link
Copy Markdown

Hi @KingsleyMcSimon . 👨‍🚀

Good job trying to solve the problems listed above.

However, we still have the same problems listed by the TSE above,
I have been checking the comments listed by the previous TSEs as well as the reference video which are a valuable resource, that you can use to resolve your method conflicts.


Please make the changes required and submit for re-review once you are ready!


Status: Changes required. ♻️


[TSE: David Elí]
Slack: @ David Eli Martinez Garcia

@KingsleyMcSimon
Copy link
Copy Markdown
Owner Author

I have actually worked on the recommended changes on the my_any?, my_none? and my_inject methods.

@sumancrest0001
Copy link
Copy Markdown

Status: Changes Required ⚙️
Hello @KingsleyMcSimon, You have done a good job to make some of the custom enumerable methods behave similarly to the original methods. I know it is a tricky project and I really appreciate your effort. We need some changes with some of the methods. Let's have a look at them and make necessary changes.

#my_each

  • let's test the method with a range and pass a block as an argument. It should return the range itself.
  • when a block is given when self is a range behaves like #each calls the given block once for each element in self
  • let's test the method with a hash and pass a block as an argument. It should return the hash itself.

#my_each_with_index
consider the following scenario

  • when self is a range and pass a block as an argument.
  • when block is given when self is a range behaves like #each_with_index calls the given block once for each element in self
  • when a block is given when self is a hash

#my_select
let's test our method with the following conditions

  • an array containing all elements of enum for which the given block returns

#my_all?

  • it should return false when the block returns false or nil.

#my_any?

  • it should return false when the block never returns false or nil.
  • when a class is passed as an argument returns true if at least one of the collections is a member. for example array.my_any?(Numeric), it should return true if at least one element is Numeric.
  • when a Regex is passed as an argument returns true if any of the collection matches the Regex

#my_none?

  • returns false if the block ever returns true for all elements
  • when a Regex is passed as an argument returns false if any of the collection matches the Regex

#my_count

  • test the method with a range also.

#my_map

  • returns a new array with the results of running a given block once for every element in enum::range
  • It should execute proc when proc and block both are given.

#my_inject

  • test the method when no block or argument is given.
  • when proc is given
  • when a block is given without an initial value combines all elements of enum by applying a binary operation, specified by a block::range
  • when a block is given with an initial value combines all elements of enum by applying a binary operation, specified by a block::range
  • when symbol is passed without an initial value
  • when symbol is passed with an initial value.

Copy link
Copy Markdown

@sumancrest0001 sumancrest0001 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please request another code review once you make the necessary changes.
if you need support or have a query regarding the review, message me on slack (Suman Shrestha)
Happy coding

@KingsleyMcSimon
Copy link
Copy Markdown
Owner Author

Hi, good afternoon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants